Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write Values to PI #540

Closed
wants to merge 3 commits into from
Closed

Write Values to PI #540

wants to merge 3 commits into from

Conversation

ldariva
Copy link

@ldariva ldariva commented Aug 6, 2020

I implemented a call to UpdateValue method which allows to send values to PI System. I tested this feature using a PI server in my company. I teste the code in Python 2.7.18, 3.5.4, 3.6.0 and 3.7.8. I also create two new enumartion. One for update option and another one for buffer option.

@ldariva
Copy link
Author

ldariva commented Aug 6, 2020

What this checks do?

@ldariva ldariva closed this Aug 8, 2020
@ldariva ldariva reopened this Aug 8, 2020
Copy link
Contributor

@AlcibiadesCleinias AlcibiadesCleinias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, I appreciate your contribution.
I left some comments. Hope it may help.

Now I really time dependent and want to have some of the features you suggested as soon as possible, so that is why I started a new branch and it consists of 80% of your pr suggestions: #573

@@ -93,7 +92,7 @@ def timestamp_to_index(timestamp):

Move outside as separate function?
"""
local_tz = pytz.timezone(PIConfig.DEFAULT_TIMEZONE)
local_tz = pytz.timezone("Europe/Amsterdam")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoded timezone

points = server.search(tag)[0]
# teste write

points.update_value(1.0,UpdateOption.REPLACE,BufferOption.BUFFERIFPOSSIBLE)
Copy link
Contributor

@AlcibiadesCleinias AlcibiadesCleinias Jul 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about the correct use of IntEnum here.

UpdateOption.REPLACE will return <enum 'UpdateMode'>.
I suggest you to use .value attr. in addition, i.e. UpdateOption.REPLACE.value

@@ -370,33 +344,6 @@ def summaries(
"""summaries

Return one or more summary values for each interval within a time range

Args:
Copy link
Contributor

@AlcibiadesCleinias AlcibiadesCleinias Jul 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot of useful comment removals

@Hugovdberg
Copy link
Owner

I think what happened is that @ldariva based his changes on the latest master branch or an older state of develop, and not on a recent commit to develop. I'm not sure how to remotely fix this. Perhaps if @ldariva could try to do the following we can get to a usefull pull request. As it stands right now I don't want to merge it, as several improvements from develop are reverted with these changes.

git pull origin develop # or perhaps git pull upstream develop. depending on how you've configured the git remotes
git rebase develop

@ldariva
Copy link
Author

ldariva commented Jul 27, 2021

Hi Hugo,

I will ty that but I am not sure this is the problem. I took a log from the repository I worked with and this is the result.

commit b0258e7 (HEAD -> develop, origin/develop, origin/HEAD)
Author: ldariva [email protected]
Date: Wed Aug 5 22:01:19 2020 -0300

Adjusted update_value.rst

commit 094f507
Author: ALEX RIGON MULLER [email protected]
Date: Wed Aug 5 21:44:42 2020 -0300

Moved AFValue creation inside update_value method.

commit e52a57b
Author: ldariva [email protected]
Date: Wed Aug 5 12:12:51 2020 -0300

Added update_value feature to allow writting to PI

commit fed90e1
Merge: 985c93d d5fec92
Author: Hugo Lapr [email protected]
Date: Fri Mar 20 11:10:05 2020 +0100

Merge pull request #524 from Hugovdberg/pyup/scheduled-update-2020-03-16

Scheduled biweekly dependency update for week 11

commit d5fec92
Author: Hugo Lapr [email protected]
Date: Fri Mar 20 11:06:10 2020 +0100
:

@AlcibiadesCleinias AlcibiadesCleinias mentioned this pull request Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants